Conversation
|
Great work 👍 Something we should consider is to use names like |
|
@jeronimoalbi Yes, we're already storing it that way. The only difference is that the Username component will render it as a styled UI badge. To tag someone, you’ll need to use the Btw we also need to update the |
|
The PR is ready for review. Supporting tagging other users while posting will be the next step. One thing to note before deploying: we’ll propably need to introduce migrations using Drizzle (https://orm.drizzle.team/docs/migrations). Currently, we're just pushing the schema directly to the database (https://github.com/allinbits/dither.chat/blob/main/packages/api-main/package.json#L17). |
| * matches the code verified against the proof tweet. | ||
| */ | ||
| export function getSocialProofCode(address: string): string { | ||
| const withoutPrefix = address.replace(/^[a-z]+1/, ''); |
There was a problem hiding this comment.
Why are you using a regular expression instead of slice(5)?
| export async function LinkSocial(action: ActionWithData): Promise<ResponseStatus> { | ||
| try { | ||
| const [username, platform, proofUrl] = extractMemoContent(action.memo, 'dither.LinkSocial'); | ||
| const postBody: { hash: string; from: string; username: string; platform: string; proof_url: string; timestamp: string } = { |
There was a problem hiding this comment.
| const postBody: { hash: string; from: string; username: string; platform: string; proof_url: string; timestamp: string } = { | |
| const postBody: Posts.SocialProofBody = { |
| /* eslint-disable ts/no-namespace */ | ||
| import type { ActionWithData, ResponseStatus } from '../types/index'; |
There was a problem hiding this comment.
Importing it allows to use Posts.SocialProofBody in the next comment:
| /* eslint-disable ts/no-namespace */ | |
| import type { ActionWithData, ResponseStatus } from '../types/index'; | |
| /* eslint-disable ts/no-namespace */ | |
| import type { Posts } from '@atomone/dither-api-types'; | |
| import type { ActionWithData, ResponseStatus } from '../types/index'; |
The space in between the imports is for consistency with all of the other files in the same folder than this one.
| ## Social Verification | ||
|
|
||
| Social username verification is asynchronous: |
There was a problem hiding this comment.
| ## Social Verification | |
| Social username verification is asynchronous: | |
| ## Social Verification | |
| Verification is done to link an AtomOne address to one or more social account usernames. | |
| Social username verification is asynchronous: |
| hash: action.hash, | ||
| from: action.sender, | ||
| username, | ||
| platform, |
There was a problem hiding this comment.
We should validate that platform is valid before anything else I think, if it fails we could send an internal message to the user.
| if (tokenAddress && tokenAddress.toLowerCase() === address) { | ||
| isOwner = true; | ||
| } |
There was a problem hiding this comment.
Maybe?
| if (tokenAddress && tokenAddress.toLowerCase() === address) { | |
| isOwner = true; | |
| } | |
| isOwner = (tokenAddress && tokenAddress.toLowerCase() === address) |
| // Verification runs in background, we don't want to block the API response. | ||
| // Future improvement: if we had a job queue system, we could push a job here instead of this. | ||
| verifyLink(insertedId, body.platform.toLowerCase(), body.proof_url, body.from.toLowerCase(), body.username.toLowerCase()).catch( | ||
| err => console.error(`verifySocialLink error for id=${insertedId}:`, err), |
There was a problem hiding this comment.
It would be a good idea to send a notification to the user with the failure reason, otherwise it might happen that verification fails and user ends up only seeing the social link pending for a long time.
There was a problem hiding this comment.
It might be worth considering maybe using a scheduled GitHub CI workflow that runs from time to time and tries to verify accounts created a while ago that failed to be verified within the max retries threshold.
| const MAX_RETRIES = 3; | ||
| const INITIAL_DELAY_MS = 2000; // 2 seconds, doubles each retry |
There was a problem hiding this comment.
WDYT about increasing max retries to 7 or 10 retries?
Also maybe even increasing the initial delay a bit. Increasing both has the potential to work if there are long hickups with a service.
| * Never throws — all errors are caught and result in a 'failed' DB update. | ||
| */ | ||
| export async function verifyLink( | ||
| id: number, |
There was a problem hiding this comment.
Maybe change id to socialLinkId for clarity?
| if (await isHandleAlreadyClaimed(username, platform, id)) { | ||
| await getDatabase() | ||
| .update(SocialLinksTable) | ||
| .set({ status: STATUS_FAILED, error_reason: ERROR_REASON_HANDLE_ALREADY_CLAIMED }) | ||
| .where(eq(SocialLinksTable.id, id)); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This should provably be the first thing to do for both cases when this function is called to make sure handle is not claimed before doing anything else. It would remove the duplicated code too.
| eq(SocialLinksTable.handle, handle), | ||
| eq(SocialLinksTable.platform, platform), | ||
| eq(SocialLinksTable.status, STATUS_VERIFIED), | ||
| ne(SocialLinksTable.id, excludeId), |
There was a problem hiding this comment.
Is the excludeId really needed? If the handle is already claimed it wouldn't matter who did it, isn't it?
It's definitely needed 👍 |
| return providers.find(p => p.id === platform.value) ?? null; | ||
| }); | ||
|
|
||
| const display = computed(() => handle.value ? `@${handle.value}` : shorten(props.userAddress || '...............', 8, 4)); |
There was a problem hiding this comment.
The "@" prefix could be removed, otherwise username would look like @elon@x:
| const display = computed(() => handle.value ? `@${handle.value}` : shorten(props.userAddress || '...............', 8, 4)); | |
| const display = computed(() => handle.value ? `${handle.value}` : shorten(props.userAddress || '...............', 8, 4)); |
|
|
||
| <Tabs v-if="wallet.loggedIn.value" :tabs="tabs" layout="fill" class="border-t" /> | ||
| <!-- Social Accounts --> | ||
| <SocialAccountsPanel |
There was a problem hiding this comment.
WDYT about moving the verification forms to settings? It seems a bit off in the user profile page.
Profile could display the already verified accounts only.
Summary
Fixes #508
Dither.Demo.mp4
Screenshots